Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add utility to migrate settings #711

Merged
merged 22 commits into from
Feb 28, 2024
Merged

Add utility to migrate settings #711

merged 22 commits into from
Feb 28, 2024

Conversation

Sidsector9
Copy link
Member

@Sidsector9 Sidsector9 commented Feb 12, 2024 •
edited by iamdharmesh
Loading

Description of the Change

PR adds migration script to migrate existing configured settings to new feature first settings introduced in classifAI v3.

Closes #670

How to test the Change

  1. Clone this repo
  2. Checkout to trunk branch, and build the plugin files
  3. Activate the plugin and configure all of the AI providers.
  4. Checkout to this PR branch. upkeep/670
  5. Verify that you see the notice about ClassifiAI settings migrated and changes in v3
  6. Go to Tools > ClassifAI and make sure all settings migrated properly.
  7. Validate each features are working properly.

Changelog Entry

Added - Tool to migrate feature settings from versions < 3.0.0

Credits

Props @dkotter @Sidsector9 @iamdharmesh

Checklist:

  • I agree to follow this project's Code of Conduct.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests pass.

@Sidsector9 Sidsector9 requested review from dkotter, jeffpaul and a team as code owners February 12, 2024 13:26
@Sidsector9 Sidsector9 self-assigned this Feb 12, 2024
@Sidsector9 Sidsector9 removed request for a team, dkotter and jeffpaul February 12, 2024 13:26
@github-actions github-actions bot added this to the 3.0.0 milestone Feb 12, 2024
@github-actions github-actions bot added the needs:feedback This requires reporter feedback to better understand the request. label Feb 12, 2024
@github-actions GitHub Actions
Copy link

@Sidsector9 thanks for the PR! Could you please fill out the PR template with description, changelog, and credits information so that we can properly review and merge this?

@dkotter dkotter mentioned this pull request Feb 20, 2024
Closed
19 tasks
@iamdharmesh iamdharmesh self-assigned this Feb 21, 2024
@iamdharmesh iamdharmesh changed the title [WIP] upkeep/670: Add utility to migrate settings upkeep/670: Add utility to migrate settings Feb 21, 2024
@github-actions github-actions bot added needs:code-review This requires code review. and removed needs:feedback This requires reporter feedback to better understand the request. labels Feb 21, 2024
@iamdharmesh
Copy link
Member

Note: E2E tests are failing due to some issue in wp-env.

@qasumitbagthariya
Copy link

QA Update ⚠️

@iamdharmesh Thanks for the PR.

I have checked migration with branch upkeep/670 and I'm getting some issue with migration.

I tried the same steps mentioned in the PR description and noticed that sometimes the migration is not working as expected.

Please check the video below, and let me know if you need any help to reproduce the issue.

Issue video:
https://drive.google.com/file/d/1YTHY4U0qRHKRnqH6kcpLnGdLbQo_3WxV/view?usp=drive_link

dkotter
dkotter reviewed Feb 26, 2024
View reviewed changes
includes/Classifai/Admin/Notifications.php Outdated Show resolved Hide resolved
includes/Classifai/Plugin.php Outdated Show resolved Hide resolved
includes/Classifai/Plugin.php Show resolved Hide resolved
@dkotter dkotter changed the title upkeep/670: Add utility to migrate settings Add utility to migrate settings Feb 27, 2024
dkotter
dkotter approved these changes Feb 27, 2024
View reviewed changes
Copy link
Collaborator

@dkotter dkotter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested this and all seemed to work fine for me. There is potential for some confusion as we've rearranged some settings and changed how the role and user based access works (no longer showing extra checkboxes) but that all looks fine to me and the main things (like API keys) all come over correctly

@dkotter
Copy link
Collaborator

dkotter commented Feb 27, 2024

One other thing to consider is we aren't deleting any of the old settings that are stored in options. I think that's okay for now, as it allows users to access that if something does go wrong in the migration process. But we probably want to look at cleaning that up in a release or two.

@dkotter dkotter merged commit b7b5f82 into develop Feb 28, 2024
13 checks passed
@dkotter dkotter deleted the upkeep/670 branch February 28, 2024 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dkotter dkotter dkotter approved these changes

Assignees

@iamdharmesh iamdharmesh

@Sidsector9 Sidsector9

Labels
needs:code-review This requires code review.
Projects
None yet
Milestone
3.0.0
Development

Successfully merging this pull request may close these issues.

Add upgrade migration routine for settings
4 participants
@Sidsector9 @iamdharmesh @qasumitbagthariya @dkotter